S101 is the only differing config between this and others (mysql-server, pg*), and it's related to the usage of assert's in the code.
@carlcsaposs-canonical I would prefer removing those, but I've added the ignore to get your position.

charms aren't run with -O, so asserts should always run

I think asserts are valuable for

• documenting assumptions that should never get raised
• they are much shorter than raising exceptions

I think adding s101 will result in less assumptions getting documented (an assert is much cheaper than if: raise)

yes we should raise exceptions (and not assert) for expected errors, but I don't think we should get rid of assert for documenting assumptions

ctrl-f "assert" in https://github.com/canonical/charm-refresh/blob/main/charm_refresh/_main.py for some examples of things I wouldn't bother writing code to raise an exception for but is worth writing an assert to document the assumption

My 2 cents is that asserts in production code is weird, due:

they are much shorter than raising exceptions

Short code is not something to pursue and I disregard it as a good argument. We should pursue being explicit instead.

documenting assumptions that should never get raised

Being explicit does a better job and, assert's will result in lost of granularity for error catching as all errors will be AssertionError

assert's will result in lost of granularity for error catching

imo we should never be catching asserts—if we're catching, a proper exception should be raised

Short code is not something to pursue

I think it's significantly more readable and, practically speaking, the alternative would be to not document assumptions at all instead of raising exceptions
for a concrete example, I think the refresh code linked above would be a lot harder to follow if the asserts were replaced with if: raise—assert makes it clear that it's validating an assumption instead of an expected error case and it keeps the "business logic" clear/front and center

👋🏻 My 2 cents about this topic is that we should avoid using assertions.

I agree that the assertions are shorten than properly defined + raised exceptions, but I think we should strive for the latter and do not take shortcuts. Both the name and doc-string of the substitute exceptions will replace the documentation value of the assertions, possibly even surpassing it.

In short: I am in favor of applying S101.

looks like this causes a lot of false positives, don't think it's worth the hassle
e.g. https://github.com/search?q=repo%3Acanonical%2Fmysql-operator%20s105&type=code

👋🏻 My 2 cents about this topic is that the S linting rule should stay, as there are a bunch of security related misconfigurations it could detect. I recognize S105 is annoying, but I think the benefits outweigh the drawbacks.

imo, many of these rules decrease readability of the code

👋🏻 My 2 cents about this topic is that the SIM linting rule should not stay, as some of the rules enforce syntax that we may not want to apply, either for readability purposes, or because we want to sneak a comment right in the middle of two statements.

It it up to you if you want to fine-tune the rules and select only those that are overwhelmingly benign (i.e. double-negation, expr-with-true-false...)

b011 for same reason as s101
b903 class with only __init__ shouldn't always be a dataclass
b904 unnecessarily verbose, default behavior of exception chaining (when from omitted) is almost always desired
ruf005 imo premature optimization; sometimes less readable
ruf010 imo decreases readability

👋🏻 My 2 cents here highly varies from rule to rule:

• B011 - Unnecessary if S101 gets apply, which I am in favour of.
• B903 - No preference.
• B904 - No preference, although if exception names are fine-grained enough, I will say unnecessay.
• RUF005 - I like it, and find it more readable than concatenating using +.
• RUF010 - No preference.

change behavior of ruf031 to be more readable

charms aren't run with -O, so asserts should always run

I think asserts are valuable for

• documenting assumptions that should never get raised
• they are much shorter than raising exceptions

I think adding s101 will result in less assumptions getting documented (an assert is much cheaper than if: raise)

yes we should raise exceptions (and not assert) for expected errors, but I don't think we should get rid of assert for documenting assumptions

ctrl-f "assert" in https://github.com/canonical/charm-refresh/blob/main/charm_refresh/_main.py for some examples of things I wouldn't bother writing code to raise an exception for but is worth writing an assert to document the assumption